Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

util, executor: use UnionRanges build index kv range for INLJ #15727

Merged
merged 3 commits into from
Mar 27, 2020

Conversation

XuHuaiyu
Copy link
Contributor

@XuHuaiyu XuHuaiyu commented Mar 26, 2020

What problem does this PR solve?

Issue Number: close #15686
close #15693
close #15687

Problem Summary:
Before this commit, we build a wrong kv ranges which will cause data lost when index join fetchInnerResult.

What is changed and how it works?

What's Changed:
Use UnionRanges to merge ranges.

How it Works:

Related changes

  • Need to cherry-pick to the release branch

Check List

Tests

  • Integration test

Side effects

  • Performance regression
    • Consumes more CPU

Release note

@XuHuaiyu XuHuaiyu added type/bugfix This PR fixes a bug. sig/execution SIG execution labels Mar 26, 2020
@XuHuaiyu XuHuaiyu requested a review from winoros March 26, 2020 08:48
@XuHuaiyu XuHuaiyu requested review from a team as code owners March 26, 2020 08:48
@ghost ghost requested review from SunRunAway and eurekaka and removed request for a team March 26, 2020 08:48
@github-actions github-actions bot added the sig/planner SIG: Planner label Mar 26, 2020
@XuHuaiyu XuHuaiyu requested a review from fzhedu March 26, 2020 09:00
@XuHuaiyu
Copy link
Contributor Author

PTAL @winoros @eurekaka

@XuHuaiyu XuHuaiyu added priority/release-blocker This issue blocks a release. Please solve it ASAP. status/PTAL labels Mar 26, 2020
@XuHuaiyu XuHuaiyu force-pushed the issue15686 branch 2 times, most recently from 05c57bf to 13dd5bb Compare March 26, 2020 13:27
@codecov
Copy link

codecov bot commented Mar 26, 2020

Codecov Report

Merging #15727 into master will not change coverage by %.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #15727   +/-   ##
===========================================
  Coverage   80.4907%   80.4907%           
===========================================
  Files           504        504           
  Lines        134608     134608           
===========================================
  Hits         108347     108347           
  Misses        17789      17789           
  Partials       8472       8472           

Copy link
Contributor

@eurekaka eurekaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

util/ranger/ranger.go Outdated Show resolved Hide resolved
Copy link
Member

@winoros winoros left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Co-Authored-By: Kenan Yao <cauchy1992@gmail.com>
@winoros
Copy link
Member

winoros commented Mar 26, 2020

I cannot figure out why the old codes is wrong since the merge algo is the same. Can you create a issue for future opotimization?

@XuHuaiyu
Copy link
Contributor Author

/merge

@sre-bot sre-bot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 27, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Mar 27, 2020

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Mar 27, 2020

@XuHuaiyu merge failed.

@XuHuaiyu
Copy link
Contributor Author

unit test failed because of #15594

@XuHuaiyu XuHuaiyu changed the title util, executor: use UnionRanges in when build kvRange for IndexReader in INLJ util, executor: use UnionRanges build index kv range for INLJ Mar 27, 2020
@XuHuaiyu XuHuaiyu merged commit 15c3b74 into pingcap:master Mar 27, 2020
@XuHuaiyu XuHuaiyu deleted the issue15686 branch March 27, 2020 02:21
sre-bot pushed a commit to sre-bot/tidb that referenced this pull request Mar 27, 2020
Signed-off-by: sre-bot <sre-bot@pingcap.com>
@sre-bot
Copy link
Contributor

sre-bot commented Mar 27, 2020

cherry pick to release-3.0 in PR #15753

sre-bot pushed a commit to sre-bot/tidb that referenced this pull request Mar 27, 2020
Signed-off-by: sre-bot <sre-bot@pingcap.com>
@sre-bot
Copy link
Contributor

sre-bot commented Mar 27, 2020

cherry pick to release-3.1 in PR #15754

@sre-bot
Copy link
Contributor

sre-bot commented Mar 27, 2020

cherry pick to release-4.0 in PR #15756

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority/release-blocker This issue blocks a release. Please solve it ASAP. sig/execution SIG execution sig/planner SIG: Planner status/can-merge Indicates a PR has been approved by a committer. type/bugfix This PR fixes a bug.
Projects
None yet
4 participants